fix: honor error_handler replacement requests on SessionError and preserve forefront on retries by Ayush7614 · Pull Request #2105 · apify/crawlee-python

Ayush7614 · 2026-07-30T14:20:18Z

Summary

BasicCrawler SessionError path: honor error_handler replacement requests only while session rotations remain; treat same unique_key as no replacement (avoids marking the in-progress request handled while add_request is a no-op); wrap handler exceptions in UserDefinedErrorHandlerError and set RequestState.ERROR when the handler raises or rotations are exhausted.
Shared _handle_error_handler_replacement helper for the SessionError and normal retry paths: inherits retry_count / session_rotation_count, passes forefront= to add_request, records record_request_processing_finish on the replaced original, and retires blocked sessions before awaits on the SessionError path.
Retries: reclaim_request passes forefront=request.forefront so tiered-proxy priority retries stay at the front of the queue.
session.retire() stays on the retry/rotation path only (preserves long-lived sessions when max_session_rotations=0).

Why

error_handler could replace a request on normal failures, but its return value was discarded for SessionError. A field-wise new_request != request check treated same-URL replacements as new work and could drop the original. Tiered proxies set request.forefront = True on retry, but reclaim always used the default forefront=False.

Test plan

test_session_error_handler_can_replace_request
test_session_error_handler_same_unique_key_rotates
test_session_error_handler_replacement_ignored_when_rotations_exhausted
test_session_error_handler_exception_is_wrapped
test_reclaim_uses_request_forefront_flag
Existing session rotation / error_handler tests still pass

Honor error_handler replacement requests for SessionError, retire blocked sessions when rotations are exhausted, propagate AdaptivePlaywright static SessionError for rotation instead of browser fallback, and reclaim retries with request.forefront for tiered-proxy priority.

Copilot

🟡 Not ready to approve

The SessionError path currently leaves requests in an inconsistent lifecycle state and misses retry/error tracking in one replacement branch, which can lead to incorrect persisted request metadata and statistics.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.

Pull request overview

This PR completes the SessionError lifecycle in BasicCrawler/AdaptivePlaywrightCrawler and ensures retry reclaiming respects request queue priority (forefront) so tiered-proxy retries stay at the front of the queue.

Changes:

Honor error_handler return values for SessionError, wrap handler exceptions consistently, and retire sessions when rotations are exhausted.
Re-raise SessionError from the adaptive crawler’s static path to trigger session rotation instead of falling through to the browser.
Pass forefront=request.forefront into reclaim_request so priority retries preserve queue ordering.

File summaries

File	Description
`src/crawlee/crawlers/_basic/_basic_crawler.py`	Updates retry reclaim behavior to honor `forefront`, and refines `SessionError` handling (rotation/retire + error_handler honoring).
`src/crawlee/crawlers/_adaptive_playwright/_adaptive_playwright_crawler.py`	Ensures static-path `SessionError` propagates to enable session rotation rather than browser fallback with the same session.
`tests/unit/crawlers/_basic/test_basic_crawler.py`	Adds unit tests covering `SessionError` error_handler replacement, session retirement on exhausted rotations, and `forefront` reclaim behavior.
`tests/unit/crawlers/_adaptive_playwright/test_adaptive_playwright_crawler.py`	Adds a unit test asserting static `SessionError` propagation triggers session rotation and prevents browser fallback.

Review details

Suppressed comments (1)

src/crawlee/crawlers/_basic/_basic_crawler.py:1504

When session rotations are exhausted, the request is marked as handled without setting its final state to ERROR. This leaves failed requests in REQUEST_HANDLER state in storage, which diverges from the normal error path (where the request is set to RequestState.ERROR before marking handled).

            else:
                # Exhausted rotations: retire the blocked session so it is not reused from the pool.
                session.retire()
                await self._mark_request_as_handled(request)

Files reviewed: 4/4 changed files
Comments generated: 2
Review effort level: Lite

We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.

Mantisus

Hey, @Ayush7614. Thank you for your contribution!

Honor error_handler replacements only while rotations remain, keep session.retire() on the retry path only, restore AdaptivePlaywright static SessionError fallback to browser, and use patch.object for forefront reclaim coverage.

Ayush7614 · 2026-08-03T13:35:28Z

Thanks for the review @Mantisus!

Addressed in 3a44e62:

Reverted the AdaptivePlaywright static SessionError re-raise — browser fallback with the same session is intentional when HTTP is blocked but JS may still work.
error_handler replacement requests are honored only inside the _should_retry_request branch, so exhausted rotations still hit failed_request_handler.
session.retire() is only called on the retry/rotation path (preserves max_session_rotations=0 long-lived sessions).
Exhausted path now sets RequestState.ERROR before marking handled.
Forefront reclaim test updated to use patch.object(..., wraps=...).

Mantisus

One nit, and please update the PR description to match the current state. Otherwise LGTM, thanks for contributing!

Assertions raised inside error_handler are wrapped as UserDefinedErrorHandlerError, so remove the isinstance check.

Ayush7614 · 2026-08-03T19:54:19Z

Thanks @Mantisus — addressed in 20ccb2b:

removed the assert isinstance(error, SessionError) from the error_handler test
updated the PR description to match the current scope

Pydantic field-wise request inequality made same-URL replacements mark the in-progress original handled while add_request was a no-op. Share the replacement path, inherit retry counters, finish stats, and retire blocked sessions before awaits.

Ayush7614 · 2026-08-04T11:07:27Z

Thanks @vdusek

Addressed in a138011:

Treat new_request.unique_key == request.unique_key as no replacement (same for the normal retry path via a shared _handle_error_handler_replacement helper).
Helper inherits retry_count / session_rotation_count, uses add_request(..., forefront=), calls record_request_processing_finish on the replaced original, sets RequestState.ERROR when the handler raises, and retires the session before awaits on the SessionError path.
Added test_session_error_handler_same_unique_key_rotates and test_session_error_handler_exception_is_wrapped.
Simplified the forefront reclaim assertion and dropped unnecessary queue.drop() calls.

vdusek

LGTM

vdusek requested review from Mantisus and Copilot July 31, 2026 09:03

Copilot started reviewing on behalf of vdusek July 31, 2026 09:04 View session

Copilot AI reviewed Jul 31, 2026

View reviewed changes

Comment thread src/crawlee/crawlers/_basic/_basic_crawler.py Outdated

Comment thread src/crawlee/crawlers/_basic/_basic_crawler.py Outdated

Mantisus suggested changes Aug 2, 2026

View reviewed changes

fix: address SessionError review feedback

3a44e62

Honor error_handler replacements only while rotations remain, keep session.retire() on the retry path only, restore AdaptivePlaywright static SessionError fallback to browser, and use patch.object for forefront reclaim coverage.

Mantisus approved these changes Aug 3, 2026

View reviewed changes

Comment thread tests/unit/crawlers/_basic/test_basic_crawler.py Outdated

test: drop assert from SessionError error_handler test

20ccb2b

Assertions raised inside error_handler are wrapped as UserDefinedErrorHandlerError, so remove the isinstance check.

Mantisus requested a review from vdusek August 3, 2026 20:07

vdusek requested changes Aug 4, 2026

View reviewed changes

vdusek approved these changes Aug 4, 2026

View reviewed changes

vdusek changed the title ~~fix: complete SessionError lifecycle and honor reclaim forefront~~ fix: honor error_handler replacement requests on SessionError and preserve forefront on retries Aug 4, 2026

vdusek changed the title ~~fix: honor error_handler replacement requests on SessionError and preserve forefront on retries~~ fix: honor error_handler replacement requests on SessionError and preserve forefront on retries Aug 4, 2026

vdusek merged commit c731597 into apify:master Aug 4, 2026
34 checks passed

Uh oh!

Conversation

Ayush7614 commented Jul 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Why

Test plan

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Not ready to approve

Suppressed comments (1)

Uh oh!

Uh oh!

Uh oh!

Mantisus left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Ayush7614 commented Aug 3, 2026

Uh oh!

Mantisus left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Ayush7614 commented Aug 3, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Ayush7614 commented Aug 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

vdusek left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Ayush7614 commented Jul 30, 2026 •

edited

Loading

Ayush7614 commented Aug 4, 2026 •

edited

Loading